WWSTCERT-10225/10228/10231/10234 add driver to frient EMI devices#2730
WWSTCERT-10225/10228/10231/10234 add driver to frient EMI devices#2730marcintyminski wants to merge 9 commits intoSmartThingsCommunity:mainfrom
Conversation
|
Duplicate profile check: Passed - no duplicate profiles detected. |
|
Invitation URL: |
Test Results 72 files 489 suites 0s ⏱️ Results for commit 6b1bda5. ♻️ This comment has been updated with latest results. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against 6b1bda5 |
...s/SmartThings/zigbee-power-meter/profiles/frient-power-energy-battery-consumption-report.yml
Show resolved
Hide resolved
drivers/SmartThings/zigbee-power-meter/profiles/frient-power-energy-current-voltage.yml
Show resolved
Hide resolved
| @@ -0,0 +1,15 @@ | |||
| -- Copyright 2025 SmartThings, Inc. | |||
| attribute = SimpleMetering.attributes.CurrentSummationDelivered.ID, | ||
| minimum_interval = 5, | ||
| maximum_interval = 3600, | ||
| data_type = data_types.Uint48, |
There was a problem hiding this comment.
Does this and the above need to be Uint48?
There was a problem hiding this comment.
That's what base data type for these attruibutes is
| @@ -0,0 +1,362 @@ | |||
| -- Copyright 2025 SmartThings, Inc. | |||
| @@ -0,0 +1,8 @@ | |||
| -- Copyright 2025 SmartThings, Inc. | |||
| @@ -0,0 +1,15 @@ | |||
| -- Copyright 2025 SmartThings, Inc. | |||
| } | ||
| }, | ||
| sub_drivers = { | ||
| require("frient/EMIZB-151") |
There was a problem hiding this comment.
match formatting -> frient.EMIZB-151
| @@ -0,0 +1,340 @@ | |||
| -- Copyright 2025 SmartThings | |||
There was a problem hiding this comment.
Does not seem like you pushed the commit.
There was a problem hiding this comment.
My bad. Now it is done
| @@ -0,0 +1,303 @@ | |||
| -- Copyright 2025 SmartThings | |||
| @@ -0,0 +1,396 @@ | |||
| -- Copyright 2025 SmartThings | |||
| lazy_load_if_possible("frient"), | ||
| lazy_load_if_possible("shinasystems"), | ||
| lazy_load_if_possible("bituo"), | ||
| lazy_load_if_possible("frient/EMIZB-151") |
There was a problem hiding this comment.
If I'm not mistake, I think this should live as a subdriver within the frient init. Thoughts @greens ?
There was a problem hiding this comment.
Moved it to subdriver section
| deviceProfileName: power-energy-consumption-report | ||
| - id: "frient A/S/EMIZB-132" | ||
| deviceLabel: frient Energy Monitor | ||
| manufacturer: Develco Products A/S |
There was a problem hiding this comment.
Are these no longer necessary?
There was a problem hiding this comment.
Right. Even if new versions of the device report a different mfr string, we have to keep the old fingerprint around.
There was a problem hiding this comment.
I brought it back
| raw_value = raw_value * multiplier / divisor * 1000 | ||
|
|
||
| -- The result is already in watts, no need to multiply by 1000 | ||
| device:emit_component_event(device.profile.components['main'], capabilities.powerMeter.power({ value = raw_value, unit = "W" })) |
There was a problem hiding this comment.
when emitting for the main component, you can just use device:emit_event
There was a problem hiding this comment.
Also this seems identical to the defaults except for the default value, which you could set elsewhere (like in added) with
device:set_field(zigbee_constants.SIMPLE_METERING_DIVISOR_KEY, SIMPLE_METERING_DEFAULT_DIVISOR)
instead of overriding the default behavior
There was a problem hiding this comment.
For some reason the configuration of attribute does not work without this handler here
| device:send(ElectricalMeasurement.attributes.ACPowerDivisor:read(device)) | ||
| device:send(ElectricalMeasurement.attributes.ACPowerMultiplier:read(device)) | ||
| device:send(ElectricalMeasurement.attributes.ACVoltageMultiplier:read(device)) | ||
| device:send(ElectricalMeasurement.attributes.ACVoltageDivisor:read(device)) | ||
| device:send(ElectricalMeasurement.attributes.ACCurrentMultiplier:read(device)) | ||
| device:send(ElectricalMeasurement.attributes.ACCurrentDivisor:read(device)) |
There was a problem hiding this comment.
I would expect these should be sent already by device:refresh() since they're configured attributes.
| if divisor == 0 then | ||
| divisor = 1 | ||
| end |
There was a problem hiding this comment.
Instead of checking for 0 every time here, you could just make sure that when you set a divisor, you just throw out the value if it's 0.
| local active_power_handler = function(component) | ||
| local handler = function(driver, device, value, zb_rx) | ||
| local raw_value = value.value | ||
| -- By default emit raw value | ||
| local multiplier = device:get_field(zigbee_constants.ELECTRICAL_MEASUREMENT_MULTIPLIER_KEY) or 1 | ||
| local divisor = device:get_field(zigbee_constants.ELECTRICAL_MEASUREMENT_DIVISOR_KEY) or 1 | ||
|
|
||
| if divisor == 0 then | ||
| divisor = 1 | ||
| end | ||
|
|
||
| raw_value = raw_value * multiplier / divisor | ||
|
|
||
| device:emit_component_event(device.profile.components[component], capabilities.powerMeter.power({ value = raw_value, unit = "W" })) | ||
| end | ||
|
|
||
| return handler | ||
| end | ||
|
|
||
| local rms_voltage_handler = function(component) | ||
| local handler = function(driver, device, value, zb_rx) | ||
| local raw_value = value.value | ||
| -- By default emit raw value | ||
| local multiplier = device:get_field(zigbee_constants.ELECTRICAL_MEASUREMENT_AC_VOLTAGE_MULTIPLIER_KEY) or 1 | ||
| local divisor = device:get_field(zigbee_constants.ELECTRICAL_MEASUREMENT_AC_VOLTAGE_DIVISOR_KEY) or 1 | ||
|
|
||
| if divisor == 0 then | ||
| divisor = 1 | ||
| end | ||
|
|
||
| raw_value = raw_value * multiplier / divisor | ||
|
|
||
| device:emit_component_event(device.profile.components[component], capabilities.voltageMeasurement.voltage({ value = raw_value, unit = "V" })) | ||
| end | ||
|
|
||
| return handler | ||
| end | ||
|
|
||
| local rms_current_handler = function(component) | ||
| local handler = function(driver, device, value, zb_rx) | ||
| local raw_value = value.value | ||
| -- By default emit raw value | ||
| local multiplier = device:get_field(zigbee_constants.ELECTRICAL_MEASUREMENT_AC_CURRENT_MULTIPLIER_KEY) or 1 | ||
| local divisor = device:get_field(zigbee_constants.ELECTRICAL_MEASUREMENT_AC_CURRENT_DIVISOR_KEY) or 1 | ||
|
|
||
| if divisor == 0 then | ||
| divisor = 1 | ||
| end | ||
|
|
||
| raw_value = raw_value * multiplier / divisor | ||
|
|
||
| device:emit_component_event(device.profile.components[component], capabilities.currentMeasurement.current({ value = raw_value, unit = "A" })) | ||
| end | ||
|
|
||
| return handler | ||
| end |
There was a problem hiding this comment.
these three could be consolidated further by having the mul/div keys, unit, and the attribute as arguments as well
| raw_value = 1 | ||
| end | ||
|
|
||
| device:set_field(zigbee_constants.SIMPLE_METERING_DIVISOR_KEY, raw_value, { persist = true }) |
There was a problem hiding this comment.
I'm not seeing a lot of added value over the default handling here.
Is the device often sending these updates with the mfg-specific bit set? It shouldn't be, since this is a standard attribute.
There was a problem hiding this comment.
It did not send it often. Switched back to default handler
| { | ||
| cluster = SimpleMetering.ID, | ||
| attribute = SimpleMetering.attributes.InstantaneousDemand.ID, | ||
| minimum_interval = 5, | ||
| maximum_interval = 3600, | ||
| data_type = data_types.Int24, | ||
| reportable_change = 1 | ||
| }, |
There was a problem hiding this comment.
default reportable_change is 5. We want it to be as sensitive as possible
There was a problem hiding this comment.
We do not. We specifically increased this to 5 because of the load it was causing to have it at 1:
There was a problem hiding this comment.
Ok. Using default configuration now.
| { | ||
| cluster = ElectricalMeasurement.ID, | ||
| attribute = ElectricalMeasurement.attributes.ActivePower.ID, | ||
| minimum_interval = 5, | ||
| maximum_interval = 3600, | ||
| data_type = data_types.Int16, | ||
| reportable_change = 5 | ||
| }, |
| { | ||
| cluster = SimpleMetering.ID, | ||
| attribute = SimpleMetering.attributes.CurrentSummationDelivered.ID, | ||
| minimum_interval = 5, | ||
| maximum_interval = 3600, | ||
| data_type = data_types.Uint48, | ||
| reportable_change = 1 | ||
| }, |
| zigbee_constants.ELECTRICAL_MEASUREMENT_AC_VOLTAGE_MULTIPLIER_KEY = "_electrical_measurement_ac_voltage_multiplier" | ||
| zigbee_constants.ELECTRICAL_MEASUREMENT_AC_CURRENT_MULTIPLIER_KEY = "_electrical_measurement_ac_current_multiplier" | ||
| zigbee_constants.ELECTRICAL_MEASUREMENT_AC_VOLTAGE_DIVISOR_KEY = "_electrical_measurement_ac_voltage_divisor" | ||
| zigbee_constants.ELECTRICAL_MEASUREMENT_AC_CURRENT_DIVISOR_KEY = "_electrical_measurement_ac_current_divisor" |
There was a problem hiding this comment.
I'd feel more comfortable if you just made these strings local to this driver rather than writing to the zigbee_constants map.
There was a problem hiding this comment.
I changed it
| -- Copyright 2025 SmartThings | ||
| -- | ||
| -- Licensed under the Apache License, Version 2.0 (the "License"); | ||
| -- you may not use this file except in compliance with the License. | ||
| -- You may obtain a copy of the License at | ||
| -- | ||
| -- http://www.apache.org/licenses/LICENSE-2.0 | ||
| -- | ||
| -- Unless required by applicable law or agreed to in writing, software | ||
| -- distributed under the License is distributed on an "AS IS" BASIS, | ||
| -- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| -- See the License for the specific language governing permissions and | ||
| -- limitations under the License. |
There was a problem hiding this comment.
update this copyright/license statement to the shorter and more concise one
| local ATTRIBUTES = { | ||
| { | ||
| cluster = SimpleMetering.ID, | ||
| attribute = SimpleMetering.attributes.CurrentSummationDelivered.ID, | ||
| minimum_interval = 5, | ||
| maximum_interval = 3600, | ||
| data_type = data_types.Uint48, | ||
| reportable_change = 1 | ||
| }, | ||
| { | ||
| cluster = SimpleMetering.ID, | ||
| attribute = SimpleMetering.attributes.InstantaneousDemand.ID, | ||
| minimum_interval = 5, | ||
| maximum_interval = 3600, | ||
| data_type = data_types.Int24, | ||
| reportable_change = 1 | ||
| } | ||
| } |
There was a problem hiding this comment.
both the same as the defaults as far as I can tell
There was a problem hiding this comment.
Difference in reportable change for InstantaneousDemand. I removed the other one
| device.thread:call_with_delay(5, function() | ||
| do_refresh(self, device) | ||
| end) |
There was a problem hiding this comment.
Why do you need to call refresh 5 seconds after you just called it at the top of this function?
There was a problem hiding this comment.
Removed the first refresh. The delayed refresh also refreshes batteryVoltage for devices supporting battery, so we want to use that.
| device:configure() | ||
|
|
||
| if device:supports_capability(capabilities.battery) then | ||
| device:send(PowerConfiguration.attributes.BatteryVoltage:configure_reporting(device, 30, 21600, 1)) |
There was a problem hiding this comment.
is this not handled by device:configure already?
There was a problem hiding this comment.
It is. Removed
| device:send(PowerConfiguration.attributes.BatteryVoltage:configure_reporting(device, 30, 21600, 1)) | ||
| end | ||
| for _, fingerprint in ipairs(ZIGBEE_POWER_METER_FINGERPRINTS) do | ||
| if device:get_model() == fingerprint.model and fingerprint.preferences then |
There was a problem hiding this comment.
can't you just check for the existence of the preference itself rather than including a separate boolean in the fingerprints map?
| local function instantaneous_demand_handler(driver, device, value, zb_rx) | ||
| local raw_value = value.value | ||
| --- demand = demand received * Multipler/Divisor | ||
| local multiplier = device:get_field(zigbee_constants.SIMPLE_METERING_MULTIPLIER_KEY) or 1 | ||
| local divisor = device:get_field(zigbee_constants.SIMPLE_METERING_DIVISOR_KEY) or SIMPLE_METERING_DEFAULT_DIVISOR | ||
| if raw_value < -8388607 or raw_value >= 8388607 then | ||
| raw_value = 0 | ||
| end | ||
|
|
||
| raw_value = raw_value * multiplier / divisor * 1000 | ||
|
|
||
| local raw_value_watts = raw_value | ||
| device:emit_event_for_endpoint(zb_rx.address_header.src_endpoint.value, capabilities.powerMeter.power({ value = raw_value_watts, unit = "W" })) | ||
| end |
There was a problem hiding this comment.
This also seems functionally the same as the default behavior.
There was a problem hiding this comment.
The device sometimes sends weird values, so we added a guard to ignore these values
| device:emit_event_for_endpoint(zb_rx.address_header.src_endpoint.value, capabilities.powerMeter.power({ value = raw_value_watts, unit = "W" })) | ||
| end | ||
|
|
||
| local function energy_meter_handler(driver, device, value, zb_rx) |
There was a problem hiding this comment.
what's different between this handler and the sub-driver's version?
There was a problem hiding this comment.
It is the same story as with instantaneous_demand_handler. If we delete the function from subdriver the configuration od CurrentSummationDelivered stops working and it only refreshes when we refresh it manually.
|
|
||
| local device_init = function(self, device) | ||
| for _, fingerprint in ipairs(ZIGBEE_POWER_METER_FINGERPRINTS) do | ||
| if device:get_model() == fingerprint.model and fingerprint.battery then |
There was a problem hiding this comment.
you can just check for whether MIN_BAT exists rather than use the extra boolean
There was a problem hiding this comment.
good point. Changed
| device:send(SimpleMetering.attributes.Divisor:read(device)) | ||
| device:send(SimpleMetering.attributes.Multiplier:read(device)) |
There was a problem hiding this comment.
i believe these will be read as part of the default refresh handlers, since they're configured attributes.
There was a problem hiding this comment.
They are not included in refresh here. If we delete them, it will never read them and it will use the default value we assigned to zigbee_constants.SIMPLE_METERING_MULTIPLIER_KEY and zigbee_constants.SIMPLE_METERING_DIVISOR_KEY
| @@ -1,4 +1,4 @@ | |||
| -- Copyright 2025 SmartThings, Inc. | |||
| -- Copyright 2026 SmartThings, Inc. | |||
| @@ -1,9 +1,11 @@ | |||
| -- Copyright 2025 SmartThings, Inc. | |||
| -- Copyright 2026 SmartThings, Inc. | |||
There was a problem hiding this comment.
did you forget to push the changes?
Check all that apply
Type of Change
Checklist
Description of Change
Summary of Completed Tests